Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(logging): change slow query log message #2795

Closed
wants to merge 5 commits into from
Closed

refactor(logging): change slow query log message #2795

wants to merge 5 commits into from

Conversation

RoloEdits
Copy link

Current summary is just a truncated version of the also provided full sql statement.

New version attempts to more clearly inform user of what exactly is causing the warn log.

Addresses #2669

Current summary is just a truncated version of the also provided full sql statement.

New version attempts to more clearly inform user of what exactly is causing the warn log.

This could be considered a breaking change, but given that the summary was just a trucated full statement it should be fine.
@RoloEdits
Copy link
Author

I wonder about changing db.statement to just statement. Would this be fine to put in this PR as well?

RoloEdits and others added 4 commits October 16, 2023 19:58
Current summary is just a truncated version of the also provided full sql statement.

New version attempts to more clearly inform user of what exactly is causing the warn log.

This could be considered a breaking change, but given that the summary was just a trucated full statement it should be fine.
sqlite still makes use of the function

private_tracing_dynamic_event!(
target: "sqlx::query",
tracing_level,
summary,
summary = format!("query went over configured duration of {}ms", self.settings.slow_statements_duration.as_millis()),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As written, the message will be the same even if the query didn't exceed the threshold.

We should also not be changing the contents of these fields.

@abonander
Copy link
Collaborator

Superceded by #2880

@abonander abonander closed this Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants